Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose boost query #250

Merged
merged 4 commits into from
Apr 24, 2024
Merged

Conversation

alex-au-922
Copy link
Contributor

Added BoostQuery class with tests.

Expected usage:

@staticmethod
def boost_query(query: Query, boost: float = 1.0) -> Query:
    pass

# If we specify a boost value
Query.boost_query(BooleanQuery(...), 2)

# Default boost value is 1
Query.boost_query(BooleanQuery(...))

Test suite (✅ for expected success case, ❌ for expected failure case)

  • ✅ Normal Boost Query
  • ✅ Boost Query nested with other Query types
  • ✅ Decimal boost values
  • ✅ Default boost value (1.0)
  • ✅ Boost Query nested in another Boost Query, and the quadratic boost effect
  • ✅ Negative boost value (negative score)
  • ❌ Wrong Query type
  • ❌ Wrong Boost Value type

@alex-au-922
Copy link
Contributor Author

The feature of default boost value could be more than essential for this library. Perhaps I can remove it if it is not desirable

@cjrh
Copy link
Collaborator

cjrh commented Apr 24, 2024

I agree - I think the boost value should always be supplied by the caller.

FYI we do already have a higher-level boost mechanism here:

query = ram_index.parse_query("winter", field_boosts={"title": 2.3})

It would be helpful to add to the docs a short example of when one would prefer to use the boost_query() method instead of the field_boosts parameter in parse_query(). Perhaps when building up a chain from a Query type that is not supported by the query parser language?

@alex-au-922
Copy link
Contributor Author

I agree - I think the boost value should always be supplied by the caller.

FYI we do already have a higher-level boost mechanism here:

query = ram_index.parse_query("winter", field_boosts={"title": 2.3})

It would be helpful to add to the docs a short example of when one would prefer to use the boost_query() method instead of the field_boosts parameter in parse_query(). Perhaps when building up a chain from a Query type that is not supported by the query parser language?

I have resolved the conflicts and updated the boost query not to have default value.

There should be two more tests:

  • ✅ Boost value 0 will return the 0 score
  • ❌ Error if no boost value is supplied

@alex-au-922
Copy link
Contributor Author

For the documentation, I am thinking should we add a new section under Building and Executing Queries, perhaps we should rename Building and Executing Queries section to something like Quickstart with QueryParser?

Currently we have a lot of implemented methods regarding the issue #20 and I think we should have a proper documentation for parameters accepted and usecases.

@cjrh
Copy link
Collaborator

cjrh commented Apr 24, 2024

Currently we have a lot of implemented methods regarding the issue #20 and I think we should have a proper documentation for parameters accepted and usecases.

yep agree 💯

should we add a new section under Building and Executing Queries

Your suggestions lead me in the direction of:

  • we rename the existing Building and Executing Queries to Building and Executing Queries with the Query Parser
  • add a new tutorial called Building and Executing Queries with the Query Objects, where we can add tutorial content using the new objects.

What do you think?

We can merge this PR first anyway, and do such docs in a different PR.

@cjrh cjrh merged commit c74990a into quickwit-oss:master Apr 24, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants